Skip to content

Conversation

@JJ-Gaisler
Copy link

This PR is based on the following PR : #1896

Please see attached test case, the test checks the exception logic related to the extension and that proxy counters work as intended.

spike -m0x80000000:0x10000000,0x20000000:0x1000,0x4000:0x1000,0xB000:0x1000 --isa rv64gh_svinval_zba_zbb_zbc_zbs_zfh_zfa_zbkb_zbkx_zicsr_zifencei_sscofpmf_smstateen_smepmp_zicbom_zicntr_zihpm_svadu_sstc_zicond_smcdeleg_smcsrind_sscsrind -p1 csr_test.txt
csr_test.txt

@aswaterman
Copy link
Collaborator

@JJ-Gaisler The commit history looks messed up; several unrelated commits are included in the changelog.

@bcstrongx could I ask you to review this PR at some point (ignoring the aforementioned unrelated commits), since you authored the spec?

// looks for the select value it won't find any csr and thus throw illegal.
// The spec states that an access to any sireg* in VS mode should raise virtual if menvcfg.cde = 1.
// Most likely the motivation for this is to not leak information about which extensions the
// underlying machine has implemented.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The VIE also allows the hypervisor to emulate the state (counter, in this case) access

auto sireg3 = std::dynamic_pointer_cast<virtualized_indirect_csr_t>(csrmap.at(CSR_SIREG3));
auto sireg4 = std::dynamic_pointer_cast<virtualized_indirect_csr_t>(csrmap.at(CSR_SIREG4));
auto sireg5 = std::dynamic_pointer_cast<virtualized_indirect_csr_t>(csrmap.at(CSR_SIREG5));
auto sireg6 = std::dynamic_pointer_cast<virtualized_indirect_csr_t>(csrmap.at(CSR_SIREG6));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like addition of sireg* should depend only on Sscsrind, not Smcdeleg (or Ssccfg). I assume we'll eventually add support for things like Smctr/Ssctr, which also use sireg*.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! Thanks for reviewing. This doesn't add sireg* as a csr it only casts the existing sireg* CSRs to the correct type to be able to use them. Sireg* is not directly available in this scope which is why I retrieve them from the csrmap.

missing proxy csr and check for csr privilege in sscsring_reg_csr_t
since mireg uses the same class.
Add stateen checks for scontext/hcontext
Fix for issue: 1893
@JJ-Gaisler
Copy link
Author

@JJ-Gaisler The commit history looks messed up; several unrelated commits are included in the changelog.

@bcstrongx could I ask you to review this PR at some point (ignoring the aforementioned unrelated commits), since you authored the spec?

Hopefully this is addressed now, something in the rebase must have gone wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants